Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(platform): Create and Delete API Keys #726

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

poswalsameer
Copy link
Contributor

@poswalsameer poswalsameer commented Feb 9, 2025

User description

Description

This PR adds the feature of creating and deleting API keys. Also, added the grid to show all the available API Keys.

Mentions

@rajdip-b

Screenshots of relevant screens

Screenshot 2025-02-09 195657

keyshade-apiKey.mp4

Developer's checklist

  • My PR follows the style guidelines of this project
  • I have performed a self-check on my work

If changes are made in the code:

  • I have followed the coding guidelines
  • My changes in code generate no new warnings
  • My changes are breaking another fix/feature of the project
  • I have added test cases to show that my feature works
  • I have added relevant screenshots in my PR
  • There are no UI/UX issues

Documentation Update

  • This PR requires an update to the documentation at docs.keyshade.xyz
  • I have made the necessary updates to the documentation, or no documentation changes are required.

PR Type

Enhancement, Tests


Description

  • Added functionality to create, display, and delete API keys.

  • Introduced UI components for managing API keys, including dialogs and cards.

  • Integrated API key management with backend controllers and state management.

  • Updated SVG assets and utility functions to support new features.


Changes walkthrough 📝

Relevant files
Enhancement
index.ts
Added CrownSVG to shared SVG assets                                           

apps/platform/public/svg/shared/index.ts

  • Added CrownSVG to the exported SVG assets.
  • Updated export list to include the new SVG.
  • +3/-0     
    page.tsx
    Integrated API key management into profile page                   

    apps/platform/src/app/(main)/(settings)/settings/@profile/page.tsx

  • Integrated API key management into the profile page.
  • Added components for creating, displaying, and deleting API keys.
  • Implemented API key fetching and state management.
  • Enhanced UI with new sections for API key operations.
  • +62/-5   
    index.tsx
    Added dialog component for creating API keys                         

    apps/platform/src/components/userProfile/apiKeys/addApiKeyDialog/index.tsx

  • Created a dialog component for adding new API keys.
  • Implemented permission selection and validation for API keys.
  • Integrated with backend API for creating API keys.
  • +406/-0 
    index.tsx
    Added card component for displaying API keys                         

    apps/platform/src/components/userProfile/apiKeys/apiKeyCard/index.tsx

  • Created a card component to display API key details.
  • Added context menu for editing and deleting API keys.
  • Integrated with state management for API key selection.
  • +107/-0 
    index.tsx
    Added confirmation dialog for deleting API keys                   

    apps/platform/src/components/userProfile/apiKeys/confirmDeleteApiKey/index.tsx

  • Added a confirmation dialog for deleting API keys.
  • Integrated with backend API for API key deletion.
  • Updated state management to handle API key removal.
  • +140/-0 
    controller-instance.ts
    Added ApiKeyController to controller instance                       

    apps/platform/src/lib/controller-instance.ts

  • Added ApiKeyController to the controller instance.
  • Integrated API key management with backend controllers.
  • +10/-1   
    index.ts
    Added state management for API keys                                           

    apps/platform/src/store/index.ts

  • Added atoms for managing API key state.
  • Integrated state management for API key operations.
  • +7/-0     
    index.ts
    Exported ApiKeyController in API client                                   

    packages/api-client/src/index.ts

  • Exported ApiKeyController from the API client package.
  • Updated API client to support API key operations.
  • +3/-1     

    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • Copy link
    Contributor

    codiumai-pr-agent-free bot commented Feb 9, 2025

    PR Reviewer Guide 🔍

    (Review updated until commit 2dddd1a)

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
    🧪 No relevant tests
    🔒 Security concerns

    API Key Exposure:
    The API key preview shown in the ApiKeyCard component could potentially expose sensitive key information. Consider masking or truncating the preview value to prevent accidental exposure of API keys in the UI.

    ⚡ Recommended focus areas for review

    Input Validation

    The API key name validation only checks if empty, but should also validate length limits and allowed characters to prevent invalid API key names

    if (!newApiKeyData.apiKeyName) {
      toast.error('API Key name is required')
      return
    }
    Potential Bug

    The expiry time calculation may show negative values when API key is expired, should handle expired keys differently

      dayjs(apiKey.expiresAt).diff(dayjs(), "day") >= 1 ? (
        `${dayjs(apiKey.expiresAt).diff(dayjs(), "day")} days`
      ) : (
        `${dayjs(apiKey.expiresAt).diff(dayjs(), "hour")} hours`
      )
    ) : "Expiring"}
    Error Handling

    The error handling for API key creation only handles 409 conflict specifically, should have more comprehensive error handling for other failure cases

    if (error.statusCode === 409) {
      toast.error('API Key already exists', {
        description: (
          <p className="text-xs text-red-300">
            An API Key with the same name already exists. Please use a different one.
          </p>
        )
      })
    } else {

    Copy link
    Contributor

    codiumai-pr-agent-free bot commented Feb 9, 2025

    PR Code Suggestions ✨

    Latest suggestions up to 2dddd1a

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Validate permissions selection
    Suggestion Impact:The commit implements permission selection functionality with checkboxes and validation by requiring at least one permission to be selected via the selectedPermissions state

    code diff:

    +  const [selectedPermissions, setSelectedPermissions] = React.useState<Set<CreateApiKeyRequest["authorities"]>>(new Set())
    +
    +  const togglePermission = useCallback((permissionId: string) => {
    +    setSelectedPermissions((current) => {
    +      const newPermissions = new Set(current)
    +      if (newPermissions.has(permissionId)) {
    +        newPermissions.delete(permissionId)
    +      } else {
    +        newPermissions.add(permissionId)
    +      }
    +      return newPermissions
    +    })
    +  }, [])
    +
    +  const getGroupState = useCallback(
    +    (group: any) => {
    +      if (!group.permissions) {
    +        return selectedPermissions.has(group.id)
    +      }
    +      const groupPermissions = group.permissions.map((p) => p.id)
    +      const selectedGroupPermissions = groupPermissions.filter((p) =>
    +        selectedPermissions.has(p)
    +      )
    +
    +      if (selectedGroupPermissions.length === 0) return false
    +      if (selectedGroupPermissions.length === groupPermissions.length)
    +        return true
    +      return 'indeterminate'
    +    },
    +    [selectedPermissions]
    +  )
    +
    +  const toggleGroup = useCallback(
    +    (group: any) => {
    +      setSelectedPermissions((current) => {
    +        const newPermissions = new Set(current)
    +        if (group.permissions) {
    +          const groupState = getGroupState(group)
    +          group.permissions.forEach((permission) => {
    +            if (groupState === true) {
    +              newPermissions.delete(permission.id)
    +            } else {
    +              newPermissions.add(permission.id)
    +            }
    +          })
    +        } else {
    +          if (newPermissions.has(group.id)) {
    +            newPermissions.delete(group.id)
    +          } else {
    +            newPermissions.add(group.id)
    +          }
    +        }
    +        return newPermissions
    +      })
    +    },
    +    [getGroupState]
    +  )
     
       const handleAddApiKey = useCallback(async () => {
         setIsLoading(true)
    @@ -43,62 +350,86 @@
           toast.error('API Key name is required')
           return
         }
    -    if (!newApiKeyData.expiryDate) {
    +
    +    const expiryDate = newApiKeyData.expiryDate || '24'
    +    if (!expiryDate) {
           toast.error('Expiry Date is required')
           return
         }
     
    +    // Create a new array from selectedPermissions to ensure we have the latest state
    +    const authoritiesArray = Array.from(selectedPermissions) ?? []

    Add validation to ensure at least one permission is selected before creating an
    API key. Currently, it's possible to create an API key with no permissions.

    apps/platform/src/components/userProfile/apiKeys/addApiKeyDialog/index.tsx [211-217]

     const authoritiesArray = Array.from(selectedPermissions).filter(Boolean)
    +if (authoritiesArray.length === 0) {
    +  toast.error('At least one permission must be selected')
    +  return
    +}
     
     const request: CreateApiKeyRequest = {
       name: newApiKeyData.apiKeyName,
       expiresAfter: expiryDate as "never" | "24" | "168" | "720" | "8760",
       authorities: authoritiesArray
     }

    [To ensure code accuracy, apply this suggestion manually]

    Suggestion importance[1-10]: 9

    __

    Why: This suggestion adds crucial validation to prevent creation of API keys with no permissions, which could lead to unusable keys. This is a critical security and usability improvement.

    High
    Validate expiry date input value

    Add error handling for invalid expiry date values. The current code assumes the
    expiry date will always be valid, but it could be undefined or invalid. Add
    validation before using it in the API request.

    apps/platform/src/components/userProfile/apiKeys/addApiKeyDialog/index.tsx [202-206]

    +const validExpiryValues = ['24', '168', '720', '8760', 'never'];
     const expiryDate = newApiKeyData.expiryDate || '24';
    -if (!expiryDate) {
    -  toast.error('Expiry Date is required')
    +if (!expiryDate || !validExpiryValues.includes(expiryDate)) {
    +  toast.error('Invalid expiry date value')
       return
     }
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    __

    Why: The suggestion adds important validation to prevent invalid expiry date values from being sent to the API, which could cause runtime errors. This is a critical validation that improves the robustness of the code.

    Medium
    • Update

    Previous suggestions

    ✅ Suggestions up to commit 2dddd1a
    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Validate required permissions selection
    Suggestion Impact:The commit implements permission selection functionality with checkboxes and validation by requiring at least one permission to be selected via selectedPermissions state

    code diff:

    +  const [selectedPermissions, setSelectedPermissions] = React.useState<Set<CreateApiKeyRequest["authorities"]>>(new Set())
    +
    +  const togglePermission = useCallback((permissionId: string) => {
    +    setSelectedPermissions((current) => {
    +      const newPermissions = new Set(current)
    +      if (newPermissions.has(permissionId)) {
    +        newPermissions.delete(permissionId)
    +      } else {
    +        newPermissions.add(permissionId)
    +      }
    +      return newPermissions
    +    })
    +  }, [])
    +
    +  const getGroupState = useCallback(
    +    (group: any) => {
    +      if (!group.permissions) {
    +        return selectedPermissions.has(group.id)
    +      }
    +      const groupPermissions = group.permissions.map((p) => p.id)
    +      const selectedGroupPermissions = groupPermissions.filter((p) =>
    +        selectedPermissions.has(p)
    +      )
    +
    +      if (selectedGroupPermissions.length === 0) return false
    +      if (selectedGroupPermissions.length === groupPermissions.length)
    +        return true
    +      return 'indeterminate'
    +    },
    +    [selectedPermissions]
    +  )
    +
    +  const toggleGroup = useCallback(
    +    (group: any) => {
    +      setSelectedPermissions((current) => {
    +        const newPermissions = new Set(current)
    +        if (group.permissions) {
    +          const groupState = getGroupState(group)
    +          group.permissions.forEach((permission) => {
    +            if (groupState === true) {
    +              newPermissions.delete(permission.id)
    +            } else {
    +              newPermissions.add(permission.id)
    +            }
    +          })
    +        } else {
    +          if (newPermissions.has(group.id)) {
    +            newPermissions.delete(group.id)
    +          } else {
    +            newPermissions.add(group.id)
    +          }
    +        }
    +        return newPermissions
    +      })
    +    },
    +    [getGroupState]
    +  )
     
       const handleAddApiKey = useCallback(async () => {
         setIsLoading(true)
    @@ -43,62 +350,86 @@
           toast.error('API Key name is required')
           return
         }
    -    if (!newApiKeyData.expiryDate) {
    +
    +    const expiryDate = newApiKeyData.expiryDate || '24'
    +    if (!expiryDate) {
           toast.error('Expiry Date is required')
           return
         }
     
    +    // Create a new array from selectedPermissions to ensure we have the latest state
    +    const authoritiesArray = Array.from(selectedPermissions) ?? []

    Add validation to ensure at least one permission is selected before creating an
    API key. Currently, an API key can be created with no permissions.

    apps/platform/src/components/userProfile/apiKeys/addApiKeyDialog/index.tsx [211-217]

     const authoritiesArray = Array.from(selectedPermissions).filter(Boolean)
    +
    +if (authoritiesArray.length === 0) {
    +  toast.error('At least one permission must be selected')
    +  return
    +}
     
     const request: CreateApiKeyRequest = {
       name: newApiKeyData.apiKeyName,
       expiresAfter: expiryDate as "never" | "24" | "168" | "720" | "8760",
       authorities: authoritiesArray
     }
    Suggestion importance[1-10]: 9

    __

    Why: The suggestion adds crucial validation to prevent creating API keys with no permissions, which could lead to unusable API keys. This is a critical security and usability improvement.

    High
    Validate input value constraints

    Add error handling for invalid expiry date values. Currently, the code only
    checks if expiryDate exists but doesn't validate if it's one of the allowed
    values ("24", "168", "720", "8760", "never").

    apps/platform/src/components/userProfile/apiKeys/addApiKeyDialog/index.tsx [202-206]

     const expiryDate = newApiKeyData.expiryDate || '24';
    -if (!expiryDate) {
    -  toast.error('Expiry Date is required')
    +const validExpiryValues = ["24", "168", "720", "8760", "never"];
    +if (!expiryDate || !validExpiryValues.includes(expiryDate)) {
    +  toast.error('Invalid expiry date value')
       return
     }
    Suggestion importance[1-10]: 8

    __

    Why: The suggestion adds important validation to prevent invalid expiry date values from being submitted, which could cause runtime errors or unexpected behavior. This is a critical validation for data integrity.

    Medium
    ✅ Suggestions up to commit fe320d4
    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Fix loading state on validation error

    Add error handling for the case when newApiKeyData.expiryDate is empty before
    making the API call. Currently, the code checks for empty value but returns
    early without setting isLoading back to false.

    apps/platform/src/components/userProfile/apiKeys/addApiKeyDialog/index.tsx [46-49]

     if (!newApiKeyData.expiryDate) {
       toast.error('Expiry Date is required')
    +  setIsLoading(false)
       return
     }

    [Suggestion has been applied]

    Suggestion importance[1-10]: 7

    __

    Why: The suggestion fixes a potential UI issue where the loading state remains true after validation error, which could leave the button in a disabled state incorrectly.

    Medium
    Fix loading state on null check
    Suggestion Impact:The commit addresses the loading state issue by moving setIsLoading(true) after the null check and adding proper error handling with try/catch/finally that ensures loading state is properly managed

    code diff:

       const deleteApiKey = useCallback(async () => {
    -    setIsLoading(true)
    -
         if (selectedApiKey === null) {
           toast.error('No API Key selected', {
             description: (
    @@ -47,41 +46,47 @@
     
         const apiKeySlug = selectedApiKey.slug
     
    -    toast.loading("Deleting your API Key...")
    -    const { success, error } =
    -      await ControllerInstance.getInstance().apiKeyController.deleteApiKey(
    -        { apiKeySlug: apiKeySlug },
    -        {}
    -      )
    +    setIsLoading(true)
     
    -    toast.dismiss()
    -    if (success) {
    -      toast.success('API Key deleted successfully', {
    -        description: (
    -          <p className="text-xs text-emerald-300">
    -            The API Key has been deleted.
    -          </p>
    +    try {
    +      toast.loading('Deleting your API Key...')
    +      const { success, error } =
    +        await ControllerInstance.getInstance().apiKeyController.deleteApiKey(
    +          { apiKeySlug: apiKeySlug },
    +          {}
             )
    -      })
     
    -      // Remove the API Key from the store
    -      setApiKeys((prevApiKeys) =>
    -        prevApiKeys.filter(
    -          (apiKey) => apiKey.slug !== apiKeySlug
    +      if (success) {
    +        toast.success('API Key deleted successfully', {
    +          description: (
    +            <p className="text-xs text-emerald-300">
    +              The API Key has been deleted.
    +            </p>
    +          )
    +        })
    +
    +        // Remove the API Key from the store
    +        setApiKeys((prevApiKeys) =>
    +          prevApiKeys.filter((apiKey) => apiKey.slug !== apiKeySlug)
             )
    -      )
    -    }
    -    if (error) {
    -      toast.dismiss()
    -      toast.error('Something went wrong!', {
    -        description: (
    -          <p className="text-xs text-red-300">
    -            Something went wrong while deleting the API Key. Check console for more info.
    -          </p>
    -        )
    -      })
    +      }
    +      if (error) {
    +        toast.error('Something went wrong!', {
    +          description: (
    +            <p className="text-xs text-red-300">
    +              Something went wrong while deleting the API Key. Check console for
    +              more info.
    +            </p>
    +          )
    +        })
    +        // eslint-disable-next-line no-console -- we need to log the error
    +        console.error(error)
    +      }
    +    } catch (error) {
           // eslint-disable-next-line no-console -- we need to log the error
           console.error(error)
    +    } finally {
    +      toast.dismiss()
         }

    Add error handling for the case when selectedApiKey is null before making the
    API call. Currently, the code checks for null but returns early without setting
    isLoading back to false.

    apps/platform/src/components/userProfile/apiKeys/confirmDeleteApiKey/index.tsx [37-46]

     if (selectedApiKey === null) {
       toast.error('No API Key selected', {
         description: (
           <p className="text-xs text-red-300">
             No API Key selected. Please select an API Key.
           </p>
         )
       })
    +  setIsLoading(false)
       return
     }
    Suggestion importance[1-10]: 7

    __

    Why: The suggestion addresses a similar loading state issue where the loading indicator could remain active after a null check error, improving the user experience.

    Medium

    Comment on lines 46 to 49
    if (!newApiKeyData.expiryDate) {
    toast.error('Expiry Date is required')
    return
    }
    Copy link
    Contributor

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Suggestion: Fix loading state on validation error

    Suggested change
    if (!newApiKeyData.expiryDate) {
    toast.error('Expiry Date is required')
    return
    }
    if (!newApiKeyData.expiryDate) {
    toast.error('Expiry Date is required')
    setIsLoading(false)
    return
    }

    @rajdip-b
    Copy link
    Member

    I'll hold up this PR for a while. The designs are incomplete in figma ig. Once we have them, we can merge it.

    @poswalsameer
    Copy link
    Contributor Author

    I'll hold up this PR for a while. The designs are incomplete in figma ig. Once we have them, we can merge it.

    Sure, will add up the new things in this branch.

    Copy link
    Member

    @rajdip-b rajdip-b left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    API call changes listed in delete API key. please replicate that in create api key aswell

    @poswalsameer
    Copy link
    Contributor Author

    API call changes listed in delete API key. please replicate that in create api key aswell

    Got you.

    Copy link

    @poswalsameer, please resolve all open reviews!

    Copy link

    PR closed due to inactivity

    Copy link

    PR closed due to inactivity

    @poswalsameer poswalsameer reopened this Feb 23, 2025
    Copy link

    @poswalsameer, please resolve all open reviews; otherwise this PR will be closed after Sun Feb 16 2025 19:27:57 GMT+0000 (Coordinated Universal Time)!

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    3 participants